fix(main-red): the LTX-2.5 VAE loader read the safetensors mmap through a uint16_t* it is not allowed to form (#674) - #688
Conversation
…t16_t* it is not allowed to form (#674) `main` has been RED on `sanitize-cpu (address,undefined)` since `cefacd2d0` (#641, the LTX-2.5 landing). Baseline run 31724380111 reports one failing test with one finding: 30/423 Test #30: test_ltx2_video ...***Failed src/vllm/model_executor/models/ltx2_loader.cpp:1288:91: runtime error: load of misaligned address 0x7f3c895be129 for type 'const uint16_t', which requires 2 byte alignment REPRODUCED LOCALLY at `7965f12bf`, same site (`:1325` after the file shifted), same class, full stack through `Ltx2LoadVaeWeights` <- `Ltx2VideoEngine::Load`: ltx2_loader.cpp:1325:91: runtime error: load of misaligned address 0x7fffc81cc129 for type 'const uint16_t' ... #0 vllm::Ltx2LoadVaeWeights(...) ltx2_loader.cpp:1325 #1 vllm::multimodal::Ltx2VideoEngine::Load(...) ltx2_video.cpp:777 THE DEFECT. `StTensor::data` points into the read-only safetensors mmap at `8 + <JSON header length> + <sum of the preceding tensors' sizes>`. Not one of those three terms is required to be even, so a BF16 tensor starting on an ODD byte is an ordinary file and not a corrupt one. Forming a `const uint16_t*` there and dereferencing it is UB on every target, and a real fault on the strict-alignment ones this project builds and ships for: `build-test-cpu-arm64`, Jetson/Orin (sm_110), Thor. THE REPAIR is the seam, not a cast and not a suppression: `vt::LoadUnaligned` (include/vt/unaligned.h), which is a `std::memcpy` with no alignment precondition and compiles to the same single load where the address does happen to be aligned. That seam exists BECAUSE of #301, the first time this class red main. This is its third recurrence -- #301 (closed: cpu_ops.cpp, laguna.cpp), #627 (open: qwen3_5_weights.cpp `TransposeBf16`), and now the VAE loader -- and minimax_h3_vae_loader.cpp:87-101 already carried the repair AND its reason in prose while the new loader reached main with the cast. Nothing gates the pattern; that is recorded on #674 rather than fixed here. THE COVERAGE THAT CAUGHT IT WAS ACCIDENTAL, which is the part worth fixing. `ltx2_fixture`'s JSON header happens to land one VAE tensor on an odd byte today; a rename or a reshape in that fixture retires the coverage silently and leaves every assertion green. So this adds a case that FORCES the odd offset -- it writes the file itself and pads the counted JSON header by one space, keeping whichever of the two parities is odd -- and then ASSERTS the parity it depends on before loading: REQUIRE((reinterpret_cast<uintptr_t>(file.Get(name).data) % 2) == 1); An edit that makes the address even now fails that REQUIRE instead of passing while covering nothing. The values are chosen bf16-exact so the check is equality rather than a band: a wrong-by-one-byte read is a hard failure. RED, then GREEN, same binary, same command, `setarch -R` (this host SIGSEGVs sanitizer builds on ASLR entropy otherwise, which reads exactly like a crash in the code under test): RED new case alone, unmodified src: ltx2_loader.cpp:1325:91: runtime error: load of misaligned address 0x7ffff7fb005b ... #1 DOCTEST_ANON_FUNC_153 test_ltx2_video.cpp:1189 GREEN new case alone: 1 passed | 0 failed | assertions: 11 | 11 passed GREEN whole target: 1/1 Test #30: test_ltx2_video ... Passed 379.03 sec GREEN whole target, re-run on THIS head after rebase onto 43a6c55: 1/1 Test #30: test_ltx2_video ... Passed 157.35 sec cmake -S . -B build-sanitize -DVLLM_CPP_BUILD_TESTS=ON -DVLLM_CPP_CUDA=OFF \ -DVLLM_CPP_SANITIZE='address,undefined' UBSAN_OPTIONS=print_stacktrace=1 \ ASAN_OPTIONS=detect_leaks=1:strict_string_checks=1 VT_POOL_BYPASS=1 \ setarch -R ctest --test-dir build-sanitize -R '^test_ltx2_video$' NOT FIXED HERE, and reported rather than swept: 19 sibling sites form the same pointer over `StTensor::data` in 11 other loaders, the widest being `qwen3_dspark_weights.cpp:99`, which forms a `const int64_t*` and so needs 8-byte alignment. They are inventoried on #627, which owns the class and asks for 27B/35B/Coder golden-md5 inertness evidence this repair has no GPU to produce. Only the site that is red is touched. The other half of the `cefacd2d0` main-red, the `device-leakage` DSR ratchet, needs no change: `11cc1d589` already routed it through the platform seam. Verified at `7965f12bf` by running both CI steps verbatim -- kcuda 0, DSR 32 == baseline 32, `test_device_leakage.py` 26/26 -- with `scripts/device-leakage-baseline.json` untouched since `99b7443bd`, so it is a repair and not a raised threshold. #553 is closed with that evidence. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code]
|
Verified the alignment claim on Thor itself (aarch64, sm_110, I built a standalone reproduction of the exact shape — a bf16 payload placed at a deliberately odd offset inside a byte buffer, read once through The UB is real and reproduces on Thor: Same diagnostic class as the CI failure. The fix arm is clean under the identical sanitizer flags, and both arms produce byte-identical values (1, 2, 3, 4, -1, -2, 0.5, 0), so the memcpy seam is value-preserving here as expected. What did NOT reproduce — and this is the correction. The PR body says the cast is "a genuine fault on the strict-alignment ones this project builds and ships for:
ARMv8 permits unaligned So the accurate statement is: undefined behaviour that the sanitizer catches and that reds a required gate, not a crash waiting to happen on our aarch64 targets. That is still a fix worth landing exactly as written — UB is UB, the Two things in this PR I'd highlight as the strongest parts, unchanged by the above:
Repro and runner are throwaway; nothing was installed on the Thor host and no build tooling was added. |
…#382 (#564) Commits the spec for the ROCm head_dim=128 decode arm, the ROCm half of #382. Zero source files; the implementation is not merged and this spec says so explicitly. Every code and upstream anchor it cites was verified exact at the pinned oracle during review: bf16_decode_opt at rocm_paged_attn.hip:1684 matches its quoted snippet verbatim, the EPL static_asserts at 268/289, the launch switches at 1875-1904 with only 8/16 instantiations, the five-combination fallback dispatch at 1937-1947, the CUDA counter-claim at cuda_paged_attn.cu:321/329/2796, and CALL_CUSTOM_LAUNCHER_BLK_HEAD upstream at the pin. Review repairs landed on the branch before merge. The blocking one: the spec opened with "Landed the ROCm d=128 decode arm" when nothing had landed -- git log -S'VT_ATTN_DECODE_D128' -- src/vt/rocm/ is empty on main and rocm_paged_attn.hip still gates on d == 256 || d == 512. Merging that text would have put a false "landed" on main, where the next agent greps for the flag, finds nothing, and cannot tell "never merged" from "reverted" from "renamed". Renamed to "Result on the implementation branch" with a banner quoting the two commands that show it, and section 4 moved to future tense for the same reason. Also added the two sections AGENTS.md requires and the spec lacked -- Risks and decisions, and Stop conditions -- drawn from material already in the spec rather than invented: the bf16-tie reduction-order risk that is why the arm ships default OFF, the single-board provenance of the 3.53x, the sm_110 1.6x-SLOWER reversal recorded but deliberately unreconciled, the dangling VT_ATTN_DECODE_WMMA forward reference, and the unquantified residual #488 gap. Plus a base-SHA mismatch and an upstream line anchor that had drifted by two. The spec is honest about what it could not gate: it labels its own throughput table "indicative, not the 2-3x-idle-reproduced standard" and states that no post-change per-call oracle re-measure was run. Known-baseline failures only: windows-msvc-* are the PR-only arm (#584), and sanitize-cpu (address,undefined) is red on main itself (#674, fix pending in #688). This PR changes only Markdown.
FOLLOWING_AGENTS_PROTOCOL Mechanical rebase by the operator so a PASSED review can land. The row is one of three reds blocking every open PR in the repository, and the only thing standing between it and main was a conflict in two SHARED RECORD files -- the exact "shared file = lock" pattern AGENTS.md names. No code conflict. Both keyed records resolved BY KEY: main's version taken wholesale, this row's scoped edits reapplied, unrelated keys verified byte-identical to main. The defect this row fixes was introduced by the LTX-2.5 lane (cefacd2, #641), which this operator coordinates; the fix and its review are another session's work, and the review is a fresh one that verified the seam is a real alignment-safe memcpy rather than a silenced diagnostic. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5[1m] [claude-code] # Conflicts: # .agents/roadmap_v1.md # docs/FEATURES.md
Closes #674.
mainhas been RED onsanitize-cpu (address,undefined)sincecefacd2d0(#641, the LTX-2.5 landing). This closes it. The other half of that same red,
the
device-leakageDSR ratchet (#553), needed no change and is dealt withbelow.
What was wrong
Ltx2LoadVaeWeightsread the safetensors mmap through a pointer it is notallowed to form:
StTensor::datapoints into the read-only mapping at8 + <JSON header length> + <sum of the preceding tensors' sizes>(
safetensors_reader.h:16-23). Not one of those three terms is required to beeven, so a BF16 tensor beginning on an odd byte is an ordinary safetensors
file and not a corrupt one. Forming and dereferencing a
const uint16_t*thereis undefined behaviour on every target, and a genuine fault on the
strict-alignment ones this project builds and ships for:
build-test-cpu-arm64,Jetson/Orin (sm_110), Thor.
The fix
vt::LoadUnaligned<uint16_t>(include/vt/unaligned.h) — astd::memcpywithno alignment precondition, which compiles to the same single load where the
address does happen to be aligned. Not a cast, not a pragma, not a suppression,
and not a change to any gate.
That seam exists because of this bug class: #301 created it the first time
this red-ed
main. This is the third recurrence — #301 (closed:cpu_ops.cpp:33,laguna.cpp:1028), #627 (open:qwen3_5_weights.cpp:332),and now the VAE loader — and
minimax_h3_vae_loader.cpp:87-101already carriedboth the repair and the reason in prose while a new loader reached
mainwiththe cast anyway. Nothing greps for the pattern; that observation is recorded on
#674 and #627 rather than acted on here.
The coverage that caught it was accidental — that is fixed too
test_ltx2_videoonly reached the defect becauseltx2_fixture's JSON headerhappens to make one VAE tensor's offset odd today. A rename or a reshape in
that fixture silently retires the coverage and leaves every assertion green.
So this adds a case that forces the odd offset — it writes the file itself
and pads the counted JSON header by one space, keeping whichever of the two
parities is odd — and then asserts the parity it depends on before loading:
An edit that makes the address even now fails that
REQUIREinstead of passingwhile covering nothing. The values are chosen bf16-exact so the value check is
equality rather than a band, which makes a wrong-by-one-byte read a hard
failure. The case deliberately does not use
Workspace: writing the wholeLTX-2.5 fixture would make it depend on the very fixture whose accidental
coverage it replaces.
Evidence
RED then GREEN, same binary, same command.
setarch -Rbecause sanitizer buildsSIGSEGV instantly on this host without it (ASLR entropy vs the kernel), which
reads exactly like a crash in the code under test.
7965f12bf***Failed—ltx2_loader.cpp:1325:91: runtime error: load of misaligned address 0x7fffc81cc129 for type 'const uint16_t', stack#0 Ltx2LoadVaeWeights#1 Ltx2VideoEngine::Load ltx2_video.cpp:777#8 test_ltx2_video.cpp:2180x7ffff7fb005b(odd), stack#1 DOCTEST_ANON_FUNC_153 test_ltx2_video.cpp:1189— so the forced-offset case reproduces it on its own1 passed | 0 failed,assertions: 11 | 11 passed,Status: SUCCESS!1/1 Test #30: test_ltx2_video ... Passed 157.35 secAlso green on the rebased head:
scripts/check-device-leakage.py --report,tests/scripts/test_device_leakage.py(26/26),scripts/check-doc-checkpoint.py,scripts/check-public-doc-tables.py,and
scripts/agent-preflight.shincluding its committed-range andcommit-trailer gates.
Baseline to subtract
test_cpu_x86_llamacpp_floor— test_cpu_x86_llamacpp_floor: the contended-leg case is load-dependent — at loadavg 63 the harness exits NO_QUIET_WINDOW (4) instead of GIVING_UP (2), so the guarantee goes untested and reads as a failure of whatever diff is in flight #618 exactly: it asserts a wall-clockratio and exits
NO_QUIET_WINDOW(4) instead ofGIVING_UP(2) under load.It failed at loadavg 216 and passed on the same tree at loadavg 56, which
is the issue's own description of itself. It reads none of the four files in
this diff.
python3 -m pytest tests/scripts/ --ignore=tests/scripts/test_cpu_kernel_bench.pyreports 10 failures (
test_check_windows_portability,test_gen_vulkan_spirv,test_mlx_system_headers,test_now_render). Allten reproduce identically on a pristine extraction of the base SHA
7965f12bfwith no diff applied, so they are pre-existing onmain.windows-msvc-cpu/windows-msvc-vulkanare PR-only with nomainbaseline (test_openai_api_server crashes on Windows with STATUS_STACK_BUFFER_OVERRUN (0xC0000409), unmasked by the #512 fix #584), plus video_engine.cpp reaches Windows with unguarded POSIX stat — reddens windows-msvc-* on EVERY open PR #664.
The other half of the
cefacd2d0red: no change neededdevice-leakagewas already repaired by11cc1d589, which routed LTX-2.5'sdevice question through
CurrentPlatform().device_type()rather than taking theALLOWLIST escape. Verified rather than assumed — both CI steps run verbatim:
kcuda=0,DSR 32 == baseline 32,test_device_leakage.py26/26, withscripts/device-leakage-baseline.jsonuntouched since99b7443bd(accelerator-seam S7), so it is a repair and not a raised threshold. #553 is
closed with that evidence. Its still-open successors #659 and #660 are separate
defects and are untouched here.
Scope, and what is left owed
Only the site that is red is touched. 19 sibling sites in 11 other loaders
form the same typed pointer over
StTensor::data; the full grep is posted as aninventory on #627, which owns the class. The widest is
qwen3_dspark_weights.cpp:99, aconst int64_t*needing 8-byte alignment — UBon 7 of every 8 offsets rather than 1 of 2. The sweep is value-identical by
construction but #627 requires 27B/35B/Coder golden-md5 inertness evidence for
the shared loader, and this repair has no GPU to produce it, so it is recorded
as owed rather than done blind.
docs/FEATURES.mdrecords the resulting guarantee on the row it belongs to —"Safetensors direct load" — rather than in the LTX-2.5 row, which is already at
its entry budget.
🤖 Generated with Claude Code